-
Notifications
You must be signed in to change notification settings - Fork 80
Update make_future and train_test_split regressors handling #447
Conversation
etna/datasets/tsdataset.py
Outdated
@@ -278,7 +278,9 @@ def make_future(self, future_steps: int) -> "TSDataset": | |||
|
|||
future_dataset = df.tail(future_steps).copy(deep=True) | |||
future_dataset = future_dataset.sort_index(axis=1, level=(0, 1)) | |||
future_ts = TSDataset(future_dataset, freq=self.freq) | |||
future_ts = TSDataset(df=future_dataset, freq=self.freq) | |||
future_ts.known_future = self.regressors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we can pass known_future directly in constructor of TSDataset
and it should be exactly known_future of current class, not just regressors.
Codecov Report
@@ Coverage Diff @@
## breaking_change/regressors #447 +/- ##
=============================================================
Coverage ? 87.67%
=============================================================
Files ? 96
Lines ? 4762
Branches ? 0
=============================================================
Hits ? 4175
Misses ? 587
Partials ? 0 Continue to review full report at Codecov.
|
@@ -278,7 +279,9 @@ def make_future(self, future_steps: int) -> "TSDataset": | |||
|
|||
future_dataset = df.tail(future_steps).copy(deep=True) | |||
future_dataset = future_dataset.sort_index(axis=1, level=(0, 1)) | |||
future_ts = TSDataset(future_dataset, freq=self.freq) | |||
future_ts = TSDataset(df=future_dataset, freq=self.freq) | |||
future_ts.known_future = self.known_future |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we put this known future into the constructor of TSDataset on the previous line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't, because the _check_known_future
fails with df_exog=Null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be a comment should be added about this
IMPORTANT: Please do not create a Pull Request without creating an issue first.
Before submitting (must do checklist)
Type of Change
Proposed Changes
Related Issue
Closing issues
closes #443